-
Notifications
You must be signed in to change notification settings - Fork 30
Remove fromPunycode #380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Remove fromPunycode #380
Conversation
| installTranslator(translator); | ||
| loadTranslations(); | ||
|
|
||
| auto list = QUrl::idnWhitelist(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, removing the .com fromPunycode() hack is a big improvement! Should we move the IDN whitelist initialization to a separate function initializeIdnWhitelist() with a comment that clarifies why this is needed and how this works? Perhaps also link to https://doc.qt.io/qt-6/qurl.html#setIdnWhitelist and https://github.com/qt/qtbase/blob/dev/src/corelib/io/qurlidna.cpp#L281-L318 in the comment. Note that "fi", "lt", "lv" already exist in https://github.com/qt/qtbase/blob/dev/src/corelib/io/qurlidna.cpp#L281-L318.
Also, see the discussion at https://qt-project.atlassian.net/browse/QTBUG-95597.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the goal is to show same url as in browse.
Right now we are depending on Qt behavior and as you can see it is always not the same as in browser.
I think extension should send URL in format that is show in browser.
| QStringLiteral("MIIEAzCCA2WgAwIBAgIQOWkBW")); | ||
| } | ||
|
|
||
| void WebEidTests::fromPunycode_decodesEeDomain() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep the test to verify that "https://xn--igusnunik-p7af.ee" is shown as "\u00F5igusn\u00F5unik.ee", use URL + QUrl::PrettyDecoded directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, extension should send correctly formated url
Add ee, fi, lt, lv to allowed IDN parsing list WE2-1173 Signed-off-by: Raul Metsma <raul@metsma.ee>
Add ee, fi, lt, lv to allowed IDN parsing list
WE2-1173
Signed-off-by: Raul Metsma raul@metsma.ee